Select compose command depending on environment #2875
Conversation
Summary of ChangesHello @jpodivin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant improvement to the project's build process by making the selection of the container orchestration command dynamic. Instead of rigidly relying on Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the development environment by dynamically selecting podman-compose if it's available, falling back to docker-compose. This is a good enhancement for flexibility.
I've noticed a small typo in the pull request title ('enviornment' should be 'environment').
I've also left one suggestion in the Makefile to make the command detection more robust by using GNU Make's if function. This avoids potential issues if podman-compose is defined as a shell alias and makes the behavior more consistent.
Makefile
Outdated
| SOURCE_BRANCH ?= $(shell git branch --show-current) | ||
| CONTAINER_RUN_INTERACTIVE ?= -it | ||
| COMPOSE ?= docker-compose | ||
| COMPOSE ?= $(shell command -v podman-compose 2> /dev/null || echo docker-compose) |
There was a problem hiding this comment.
While this approach works in most cases, it can be brittle if podman-compose is defined as a shell alias. The command -v utility might output the alias definition, which is not an executable command.
A more robust approach is to check for the command's existence without using its output directly, and then assign the command name to the COMPOSE variable. This avoids issues with aliases and is also slightly more readable and idiomatic for a GNU Makefile.
COMPOSE ?= $(if $(shell command -v podman-compose 2>/dev/null),podman-compose,docker-compose)
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 47s |
jpodivin
force-pushed
the
compose-engine
branch
from
ab97f26 to
8cbdacd
Compare
jpodivin
changed the title
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 47s |
jpodivin
force-pushed
the
compose-engine
branch
from
8cbdacd to
0d8cda6
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 45s |
lbarcziova
left a comment
lbarcziova
left a comment
There was a problem hiding this comment.
lgtm, please rebase and we can merge this after
|
Why not |
Signed-off-by: Jiri Podivin <[email protected]>
jpodivin
force-pushed
the
compose-engine
branch
from
0d8cda6 to
2b54d93
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 47s |
4 participants
The
COMPOSEvar is no longer hardcoded todocker-compose. Now it will be set topodman-composepreferentially, if one is available on the system.The same approach is used in Log Detective. https://github.com/fedora-copr/logdetective/blob/main/Makefile#L4
RELEASE NOTES BEGIN
Dynamically select command for compose when using Makefile.
RELEASE NOTES END